Skip to content

fix(sync): skip projects without an absolute local path (#949)#992

Merged
groksrc merged 3 commits into
mainfrom
fix/issue-949
Jun 12, 2026
Merged

fix(sync): skip projects without an absolute local path (#949)#992
groksrc merged 3 commits into
mainfrom
fix/issue-949

Conversation

@groksrc

@groksrc groksrc commented Jun 12, 2026

Copy link
Copy Markdown
Member

Summary

Fixes #949 — a config.json project entry with an empty path (e.g. {"path": ""}) caused background sync and the watch service to adopt the process cwd as the project root, injecting Basic Memory frontmatter into unrelated markdown files in whatever directory basic-memory mcp was launched from.

Root cause

The existing guards in initialize_file_sync and WatchService._select_projects_to_watch only skipped a project when get_project_mode() returned CLOUD. But ProjectEntry.mode defaults to LOCAL, so a config entry like {"projects": {"x": {"path": ""}}} (no explicit mode) is treated as local, slips past the guard, and Path("") resolves against the current working directory.

Fix

Gate local sync and watching on the path itself rather than the project mode: any active project whose path is not absolute is excluded, regardless of mode.

  • services/initialization.py — skip non-absolute-path projects before starting background sync
  • sync/watch_service.py — same gate in _select_projects_to_watch
  • drop the now-unused ProjectMode imports

This is strictly safer than the previous behavior:

  • Legitimate local projects are always resolved to absolute paths at creation (ProjectService.add_project / move_project), so they are unaffected.
  • Cloud projects with a real local bisync copy keep their absolute path and are still synced/watched (covered by the existing test_run_keeps_cloud_projects_with_local_bisync).

Tests

  • test_run_filters_empty_path_local_mode_project — empty-path, LOCAL-default project is excluded from the watch cycle.
  • test_initialize_file_sync_skips_project_with_non_absolute_path — same for background sync, asserted via the skip log.

Verification

  • tests/sync/ — 155 passed, 3 skipped
  • tests/services/test_initialization.py — green
  • ruff format --check — clean
  • ty check — All checks passed

🤖 Generated with Claude Code

A project entry in config.json with an empty path (e.g. `{"path": ""}`)
caused background sync and the watch service to adopt the process cwd as
the project root, injecting Basic Memory frontmatter into unrelated
markdown files.

The existing guards only skipped a project when get_project_mode()
returned CLOUD. But ProjectEntry.mode defaults to LOCAL, so an empty- or
relative-path entry without an explicit mode slipped through, and
Path("") resolves against the current working directory.

Gate local sync and watching on the path itself: any project whose path
is not absolute is excluded, regardless of mode. Legitimate local
projects are always resolved to absolute paths at creation, and cloud
projects with a real local bisync copy keep their absolute path and are
still synced/watched.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Drew Cain <groksrc@gmail.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 587cc80eaa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +133 to +136
skip = [p.name for p in active_projects if not Path(p.path).is_absolute()]
if skip:
active_projects = [p for p in active_projects if p.name not in skip]
logger.info(f"Skipping projects without an absolute local path for sync: {skip}")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Exclude orphan DB projects from local sync

When reconciliation is skipped or fails, an active DB row that is no longer present in the current config now passes this path-only filter as long as its stored path is absolute. The previous mode-based guard treated unknown config projects as cloud-only (get_project_mode() defaults missing projects to CLOUD), so stale rows removed from config did not get synced/watched; after this change the background sync below can still mutate that directory even though config is the source of truth. Please keep the config-membership check before applying the absolute-path gate, and mirror the same behavior in the watch selection.

Useful? React with 👍 / 👎.

groksrc and others added 2 commits June 12, 2026 17:04
The absolute-path sync guard from the previous commit now checks every
project's path, not just cloud-mode ones. Three existing watch-selection
tests hardcoded POSIX paths like "/tmp/alpha", which are absolute on
Linux/macOS but not on Windows (no drive letter), so the guard filtered
them out and the tests failed on windows-latest.

Build the project paths from the tmp_path fixture so they are absolute on
every platform. Production paths are unaffected: real local projects are
always resolved to OS-absolute paths at creation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Drew Cain <groksrc@gmail.com>
Address Codex review feedback. The previous path-only filter dropped an
implicit protection: get_project_mode() defaults projects missing from
config to CLOUD, so the old mode-based guard skipped stale DB rows that
had been removed from config. With a path-only check, an orphan row with
an absolute path would pass and background sync/watch could still mutate
a directory the user already removed from config (config is the source of
truth) if reconciliation was skipped or failed.

Introduce BasicMemoryConfig.is_locally_syncable(name, path), which
requires both config membership and an absolute path, and use it from
both the background sync selection and the watch cycle so the two paths
cannot diverge. Add direct unit tests for the helper plus an orphan-row
regression test for the watch selection.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Drew Cain <groksrc@gmail.com>
@groksrc groksrc merged commit b997d85 into main Jun 12, 2026
23 checks passed
@groksrc groksrc deleted the fix/issue-949 branch June 12, 2026 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Project with empty path in config.json makes sync adopt the process cwd, mutating unrelated files

1 participant